-
-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Normalize debugger temp file paths on Windows #838
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than adding a dependency, we could use a ctypes
version of this function call: https://mail.python.org/pipermail/python-win32/2008-January/006642.html
@blink1073 thanks for the review! Updated to use ctypes and remove the direct dependency on pywin32. The ctypes code is loosely based on the similar handling in debugpy here. |
return filename | ||
|
||
# test that it works | ||
_convert_to_long_pathname(__file__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add this as a test in test_debugger.py
instead of inline here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add it there to test as well. The intent with adding it here is to catch any errors with e.g. DLL references early so we can fail just once up front and fallback to not normalizing, rather than failing every time we get the path below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test, and clarified the inline comment. Note that debugpy (or rather pydevd) does a similar runtime check here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
Fixes #784